Hey, thanks for the PR.

Could you explain a bit more what is actually improved by these changes? The travis configuration was working just fine to my knowledge. If there are any specific issues fixed, could you tell us what those are?

The ignored flake8 errors you are listing are explicitly ignored already here:

Regarding the new .gitignore, it seems way too large to me. Many patterns there are for files that shouldn't end up in the source tree at all (although I only glanced at it briefly).

@de-vri-es thanks your comment.
Somehow I have overlooked the flake8 configuration in setup.cfg but still, I would rather split testing and code formating into those commands pytest and flake8.
There is an added validation of setup.py and generated code coverage.
I have also cut out the required packages for package run/test so it is much easier for a user to run the testing/integration locally...
Talking about .gitignore I will drop some patterns but still, I would extend them especially if the user may run some experiments and testing locally and then accidentally finds he committed files which should not be there like coverage...

Thank you for the PR. My feedback:

I like the requirements.txt, I think it is a useful addition. This reminds me, could you configure it to mention we depend on keras-resnet==0.1.0? There is a difference between 0.1.0 and 0.2.0 that I disagree with: broadinstitute/keras-resnet#57

Hmm I prefer to keep the test settings in setup.cfg, so that you can "just" run pytest or flake8 locally without having to remember all the necessary arguments. If I understand this PR correctly then you have to call:

flake8 . --max-line-length=100 --ignore=E201,E202,E203,E221,E241,E251,E402,E501,W504

Also, what's the advantage of running coverage instead of pytest? I'm not familiar with that command but I assume it produces some coverage report. While interesting, it's not required for a CI I would argue. We simply need to know if the unit tests succeeded and if they failed, why they failed.

I agree with @de-vri-es regarding the .gitignore, I think most of the items are redundant. I would prefer to limit the addition to only the logs/ directory.

I like the requirements.txt, I think it is a useful addition. This reminds me, could you configure it to mention we depend on keras-resnet==0.1.0? There is a difference between 0.1.0 and 0.2.0 that I disagree with: broadinstitute/keras-resnet#57

sure

Hmm I prefer to keep the test settings in setup.cfg, so that you can "just" run pytest or flake8 locally without having to remember all the necessary arguments. If I understand this PR correctly then you have to call...

OK

Also, what's the advantage of running coverage instead of pytest? I'm not familiar with that command but I assume it produces some coverage report. While interesting, it's not required for a CI I would argue. We simply need to know if the unit tests succeeded and if they failed, why they failed.

it is a misunderstanding, running coverage instead of pytest... the py.test is still there... is collecting statistic what part of code was tested while running pytest co later you may introduce rule to PR that all additive changes have to be cover by existing or new tests... see https://codecov.io

I agree with @de-vri-es regarding the .gitignore, I think most of the items are redundant. I would prefer to limit the addition to only the logs/ directory.

I will have look at it...

@hgaiser see codecov - https://codecov.io/gh/Borda/keras-retinanet/tree/travis-req/keras_retinanet

@de-vri-es @hgaiser could you have a look at last update? Thx

I'm still not a fan of the huge .gitignore. However, I'll defer to @hgaiser for judgment.

@de-vri-es my understanding of the git ignore is prevent users/developer on whatever platform/IDE they work to commit unwanted files without any long verification... the original list was very short and conservative to files generating while running tests... but let me know how you wish to have it or I can drop the .gitignore from this PR completely... @hgaiser

Well, ./setup.py build/build_ext/install and pytest create some files that only need these patterns:

/.eggs
/build
/dist
/keras_retinanet.egg-info
/logs

I'm fine with adding those. The rest doesn't make sense to me. IDE and editor files should be ignored by someones global config. We shouldn't add support for every possible IDE and editor out there.

@de-vri-es I have dropped all user/IDE/platform depending ignores and kept just related to the build and testing/coverage...

Codecov Report

❗ No coverage uploaded for pull request base (master@f945c0e). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff            @@
##             master   #1104   +/-   ##
========================================
  Coverage          ?     22%           
========================================
  Files             ?      39           
  Lines             ?    2301           
  Branches          ?       0           
========================================
  Hits              ?     514           
  Misses            ?    1787           
  Partials          ?       0

Looks much better, thanks. I've got one more request though: could you add a / in front of the .gitignore patterns that should only match in the repository root? So for example: /build/ instead of build/.

added for the Distribution / packaging section

Thanks! @hgaiser: Anything left on your side?

Looks good to me. We will probably tweak the codecov settings a bit as we go, but this seems like a good start. Thanks for the contribution @Borda and @de-vri-es !